Skip to content

Improve option_and_then_some lint #5529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 17, 2020
Merged

Improve option_and_then_some lint #5529

merged 2 commits into from
May 17, 2020

Conversation

alex-700
Copy link
Contributor

fixed #5492

changelog: Improve and generalize option_and_then_some and rename it to bind_instead_of_map.

phansch
phansch reviewed Apr 25, 2020
View reviewed changes
clippy_lints/src/methods/mod.rs
/// ```
pub OPTION_AND_THEN_SOME,
pub BIND_INSTEAD_OF_MAP,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a full review, but this will need to be registered as 'renamed' here:

rust-clippy/clippy_lints/src/lib.rs

Lines 1775 to 1779 in 77c23b7

pub fn register_renamed(ls: &mut rustc_lint::LintStore) {
ls.register_renamed("clippy::stutter", "clippy::module_name_repetitions");
ls.register_renamed("clippy::new_without_default_derive", "clippy::new_without_default");
ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity");
ls.register_renamed("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.
Anyway I failed with renaming :) What should I do (or maybe undo) to fix problem with the lint link absence in CHANGELOG.md?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove the surrounding [] in line 318 of CHANGELOG, so it isn't a link anymore.

@phansch phansch added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Apr 25, 2020
@alex-700 alex-700 force-pushed the improve-option-and-then-some-lint branch 2 times, most recently from f546d30 to 5c65635 Compare April 25, 2020 21:10
@flip1995 flip1995 requested a review from phansch April 25, 2020 21:22
@flip1995 flip1995 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 25, 2020
@alex-700 alex-700 force-pushed the improve-option-and-then-some-lint branch 3 times, most recently from 38fc5b7 to abdaa7f Compare April 26, 2020 06:39
@bors
Copy link
Contributor

bors commented Apr 26, 2020

☔ The latest upstream changes (presumably #5509) made this pull request unmergeable. Please resolve the merge conflicts.

@alex-700 alex-700 force-pushed the improve-option-and-then-some-lint branch 2 times, most recently from ad329c2 to 82fcf4f Compare April 26, 2020 13:26
@bors
Copy link
Contributor

bors commented May 16, 2020

☔ The latest upstream changes (presumably #5563) made this pull request unmergeable. Please resolve the merge conflicts.

@alex-700
simplify multispan_sugg interface
cb7f967 
- add `multispan_sugg_with_applicability`
- not it gets `&str` instead of `String`, like in `diag.multispan_suggestion`
@alex-700 alex-700 force-pushed the improve-option-and-then-some-lint branch from 82fcf4f to e2c9df3 Compare May 16, 2020 21:11
phansch
phansch reviewed May 17, 2020
View reviewed changes
Copy link
Contributor

@phansch phansch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one question

tests/ui/blocks_in_if_conditions.rs
@@ -63,10 +63,10 @@ fn block_in_assert() {
let opt = Some(42);
assert!(opt
.as_ref()
.and_then(|val| {
.map(|val| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it changed because it triggered the bind_instead_of_map lint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap

@phansch
Copy link
Contributor

phansch commented May 17, 2020

@bors r+ thanks!

@bors
Copy link
Contributor

bors commented May 17, 2020

📌 Commit e2c9df3 has been approved by phansch

@bors
Copy link
Contributor

bors commented May 17, 2020

⌛ Testing commit e2c9df3 with merge 92f642c...

bors added a commit that referenced this pull request May 17, 2020
@bors
Auto merge of #5529 - alex-700:improve-option-and-then-some-lint, r=p…
92f642c 
…hansch

Improve `option_and_then_some` lint

fixed #5492

changelog: Improve and generalize `option_and_then_some` and rename it to `bind_instead_of_map`.
@bors
Copy link
Contributor

bors commented May 17, 2020

💔 Test failed - checks-action_test

@alex-700
improve and generalize option_and_then_some lint
07f1edf 
- rename it to bind_instead_of_map
@alex-700 alex-700 force-pushed the improve-option-and-then-some-lint branch from e2c9df3 to 07f1edf Compare May 17, 2020 09:17
@alex-700
Copy link
Contributor Author

@phansch , bors checks need to be retried.
It was a deprecated lint name in my tests (which was deprecated during PR review).

@phansch
Copy link
Contributor

phansch commented May 17, 2020

@bors r+

@bors
Copy link
Contributor

bors commented May 17, 2020

📌 Commit 07f1edf has been approved by phansch

@bors
Copy link
Contributor

bors commented May 17, 2020

⌛ Testing commit 07f1edf with merge af38489...

bors added a commit that referenced this pull request May 17, 2020
@bors
Auto merge of #5529 - alex-700:improve-option-and-then-some-lint, r=p…
af38489 
…hansch

Improve `option_and_then_some` lint

fixed #5492

changelog: Improve and generalize `option_and_then_some` and rename it to `bind_instead_of_map`.
@bors
Copy link
Contributor

bors commented May 17, 2020

💔 Test failed - checks-action_test

@phansch
Copy link
Contributor

phansch commented May 17, 2020

@bors retry (Connection reset by peer (os error 104))

@bors
Copy link
Contributor

bors commented May 17, 2020

⌛ Testing commit 07f1edf with merge 6ae0643...

@bors
Copy link
Contributor

bors commented May 17, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch
Pushing 6ae0643 to master...

@bors bors merged commit 6ae0643 into rust-lang:master May 17, 2020
@alex-700 alex-700 mentioned this pull request May 19, 2020
Closed
@flip1995 flip1995 mentioned this pull request May 20, 2020
Closed
@alex-700 alex-700 deleted the improve-option-and-then-some-lint branch December 29, 2020 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 left review comments

+1 more reviewer

@phansch phansch phansch left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhance/New Lint: option_and_then_some for the Result equivalent
4 participants
@alex-700 @bors @phansch @flip1995